fix(webactor): notice a worker that dies before its handshake answers - #18

Started from the CI failure on tests/worker/supervisor.test.ts > should handle async shouldRetry with Promise rejection for real worker (AssertionError: expected +0 to be 1). That turned out to be a flaky test, but tracking down why it was flaky uncovered a real gap in applyWorkerSupervisor.

The flaky tests

Three tests raced the worker startup they were meant to observe. Measured with an instrumented probe, the handshake takes 49–66 ms on an idle machine, against a hard 100 ms sleep — 35–50 ms of slack. A 2–4 core runner with 16 vitest files in parallel blows through that.

Plain repetition does not reproduce it (15/15 pass locally, idle). Shrinking the wait from 100 ms to 40 ms reproduced the exact CI error on the first try.

Fixed by waiting for the condition. Liveness assertions moved into vi.waitFor; "no second restart happened" keeps a fixed settle window, which fails open on a slow machine.

The manual-termination test had a second defect that no timeout tolerance could fix: on a slow machine its blind 300 ms timer killed the worker before the handshake completed, and the supervisor genuinely never restarts that — see below. It now terminates only after a ping/pong proves the worker is up, so it tests what it claims.

The gap

Death detection in applyWorkerSupervisor has exactly two triggers:

1. on(worker, 'error', …)
2. onUnlock(threadId, …) — but the lock key is learned from the handshake reply, so the watch can only be armed after that reply arrives.

Before that reply, only trigger 1 exists. And the handshake request has no deadline of its own: it retries every retryDelay forever, and its abort signal is fired only from close(), which is only reachable from decide(). So a worker dying quietly in the startup window was never noticed — no restart, no error, nothing.

Probed with six scenarios, counting shouldRetry calls over a 3 s window:

Note the asymmetry: after the handshake, silent death is covered by the lock watch. The library already intends to catch this — it just had a startup blind window. C and F were always caught because they surface as error events; E is the realistic production shape of the gap (host kills the worker, or it closes itself).

The change

applyWorkerSupervisor accepts getAbortSignal, consulted once per launch:

applyWorkerSupervisor(WorkerConstructor, {
    shouldRetry,
    getAbortSignal: () => AbortSignal.timeout(2000),
});

A factory rather than a plain signal because a supervisor relaunches — one signal would already be spent by the second worker. There is a test pinning that a fresh signal is built per launch.

Nothing changes when it is omitted, and there is a test pinning that too: a deadline tight enough to be useful would misfire on a loaded machine, so the choice stays with the caller.

Why a signal and not a timeout number. An earlier revision of this PR added timeout to request and openTimeout to openChannel. Both were dropped: measurement showed AbortSignal.timeout already bounded the whole of openChannel on main — both the request for the port and the handshake that follows — so those options added ergonomics, not capability. applyWorkerSupervisor was the only place with a genuine hole, because its abort controller is internal and callers had no way to reach it. Building the fix on the primitive the rest of the library already takes keeps one way to bound an operation instead of two.

The catch fix is the other half, and it is not cosmetic. The handshake used .catch(catchAbortToSymbol), which maps aborts to a sentinel and rethrows the rest — so a failure became an unhandled rejection instead of a restart decision. It now distinguishes by source rather than by reason shape: only the supervisor's own teardown stays quiet. That distinction matters — with a reason-shape check, a plain AbortController from the caller aborts with AbortError, is mistaken for teardown, and gets swallowed, leaving the worker unwatched all over again. Verified: swapping the source check for isAbort fails the test covering that case.

Verification

• Unit: 145 pass (was 140).
• Every new test was checked to be load-bearing by reverting the corresponding source change and confirming it fails.
• Probes A, D and E all reach shouldRetry with a TimeoutError when a signal is supplied, and still reach nobody when it is not.
• All 9 pre-existing tests in the supervisor file pass with worker startup artificially stalled 400 ms and 1500 ms — 15× the old budget.
• Build (tsc), oxlint, format:check clean. e2e 32/32, devtools 32/32.

Three unrelated flakes surfaced while verifying, all in tests this PR does not touch, none reproducible, and all green on main: e2e/tests/devtools-scenarios.spec.ts:394, packages/devtools tests/extension.spec.ts:62, and packages/devtools tests/panel.spec.ts:397. The last one failed the DevTools job on CI here and passed on a re-run with no code change; it survives 34 local runs including 4× parallel. It is the same defect class as the tests fixed above — a fixed 600 ms wait for a force-directed layout to settle, then a click at coordinates sampled before the click lands, so the node can drift out from under it on a slow machine. Worth its own fix, separately.

Left alone, on purpose

In the supervisor's .then(), a handshake reply without a string threadId still arms no watch and says nothing — permanently blind, silently. Out of scope here; worth a follow-up.

🤖 Generated with Claude Code